Ros2 devel #90
Ros2 devel #90
Conversation
|
friendly ping for review @machinekoder @130s |
| else() | ||
| message(STATUS "The compiler ${CMAKE_CXX_COMPILER} has no C++11 support. Please use a different C++ compiler.") | ||
| if(NOT DEFINED CMAKE_SUPPRESS_DEVELOPER_WARNINGS) | ||
| set(CMAKE_SUPPRESS_DEVELOPER_WARNINGS 1 CACHE INTERNAL "No dev warnings") |
There was a problem hiding this comment.
Just for notekeeping's sake here is what is happening here:
We were trying to upgrade ALVAR to ROS 2 Foxy (and ROS 1 Noetic). The vendored copy of ALVAR in the package used a number of OpenCV function calls that are currently deprecated. This left us with a number of ways to proceed:
(0) Manually upgrade the vendored package.
(1) Find an upgraded vendor package and shoe horn it in and proceed with the upgrade.
(2) Re-write the ALVAR implementation to use the same fiducials but gut the ALVAR library.
Options 0 and 2 were too time consuming so we went with option 1. The upgraded vendor package had been run through a linter / refactored so it caused massive, gnarly diffs. In retrospect we should have broken this up into a vendor upgrade and then a ROS upgrade. I am going to let this slide as my fault.
I skimmed the ALVAR libraries and didn't see any glaring issues; it is a cut and dried OpenCV upgrade + lint / refactor. The tests, launch files, test data, package files, etc all appear in good working order (even the launch files are tested!). To be perfectly honest I would have thrown out the Kinect support and PR2 support but hey, maybe there are people still using ten year old hardware in a lab somewhere.
I did see one pressing issue. There seems to be a set of wayward VSCode files that need to be removed. This is such a huge PR that I used comments to point these out. These files need to get removed.
As it stands I would need to burn half a day setting up a system to build / test this (I'm overdue for a system upgrade). Let's fix the VSCode lingering files, do a quick demo / walk through, and I'll appove it and see if we can't merge it in.
|
Patrick put together this first pass at a ROS 2 version of ALVAR for Rolling (it builds on Foxy with some hand holding related to perception PCL). The current issue with the package is that the vendored library (ALVAR) was built around deprecated OpenCV APIs. Patrick used an upgraded version of the vendored package and then ported everything to ROS 2. The upgraded vendor code was formatted differently hence the huge PR. We went throught and resolved my comments, built the package, and ran the tests / examples. This was a fairly heroic effort and I am not sure if it is worth doing again but this should buy users a few years. We put a deprecation note in the docs to suggest to the 14 downstream packages to look for an alternative solution. There are still a lot of build warnings that need to be addressed as well. We are hoping someone can do this as a subsequent PR. Finally, while the tests and launch files run they require some work to load the ROS 1 bags to run the tests. This feels like it could stand to be a subsequent pull request as well. TL;DR this is most of the way there but we have a few more steps to get it read for release. |
ar_track_alvar/CMakeLists.txt
Outdated
| OpenCV | ||
| tf2_ros | ||
| tf2 | ||
| pcl_ros |
There was a problem hiding this comment.
I tried run rosdep install --from-paths src --ignore-src --rosdistro foxy -yr during installation, and this is not specified in package.xml. Also realized that pcl_ros is not available as debian in foxy.
Anyway, we can also remove this since that pcl_ros is not used anyway in the code.
There was a problem hiding this comment.
Thanks @youliangtan I made the changes you suggested
ar_track_alvar/CMakeLists.txt
Outdated
| src/Util.cpp | ||
| ) | ||
|
|
||
| target_link_libraries(${PROJECT_NAME} ${OpenCV_LIBRARIES} tinyxml) |
There was a problem hiding this comment.
To make it compilable, I added find_package(PCL REQUIRED) on top, and alse appended ${PCL_LIBRARIES} here.
ros2 port of ar_track_alvar